Skip to content

feat(activesync): cut steady-state polling and export cost (#88) - #91

Merged
TDannhauer merged 2 commits into
FRAMEWORK_6_0from
feat/sync-performance
Jul 24, 2026
Merged

feat(activesync): cut steady-state polling and export cost (#88)#91
TDannhauer merged 2 commits into
FRAMEWORK_6_0from
feat/sync-performance

Conversation

@TDannhauer

Copy link
Copy Markdown
Contributor

Summary

  • Cuts the steady-state cost of the PING/heartbeat loop: one batched IMAP
    STATUS round trip per poll iteration (instead of one per folder) and
    lock-free, memoized read-only state loads (zero SQL on unchanged
    synckeys).
  • Makes SQL SyncCache saves dirty-field aware (merge instead of blob
    overwrite, skip when clean), matching the Mongo backend semantics.
  • Batches email message fetches during export via a new optional
    Driver_Base::getMessagesBulk() (10 per batch, per-id fallback).

Implements the plan from #88. Companion PR: horde/Core#212.

Motivation

See #88: a device pinging a few dozen mail folders generated hundreds of
SQL statements and IMAP round trips per minute with zero changes to
report. Robustness first: every optimization is an optional hint with an
explicit per-item fallback, and foreign-client change detection is
unchanged (fresh STATUS data every iteration, prefetch consumed once).

Changes

  • Collections::pollForChanges(): gathers the backend folder ids of all
    pinged email collections and calls the new no-op-by-default
    Driver_Base::prefetchFolderStatus() once per iteration; poll loads
    pass a new readonly flag through initCollectionState().
  • Imap_Adapter::prefetchStatus(): single multi-mailbox STATUS
    (LIST-STATUS where the server supports it), consume-once in ping(),
    per-mailbox fallback on errors or missing/incomplete entries.
    getMessages() gains a uid_keys option.
  • State_Base::loadState(): optional readonly flag — no collection
    lock, no garbage collection. State_Sql replaces SELECT … FOR UPDATE with a plain SELECT and memoizes decoded state rows per
    folder and synckey; any mutating load, state save, or synckey change
    invalidates the memo. Column metadata from Horde_Db is cached per
    table.
  • State_Sql::saveSyncCache(): honors $dirty (per-collection merge,
    removals included), reads the stored blob in the query that previously
    only counted rows, UPDATE-first then INSERT, skips clean saves, and
    logs dirty property names instead of full cache blobs. SyncCache
    gets a missing dirty mark for bodypartprefs.
  • Connector_Exporter_Sync: prefetches up to 10 upcoming CHANGE/DRAFT
    items through getMessagesBulk() (initial-sync bare-uid lists
    included); ids missing from a batch fall back to single getMessage()
    calls with unchanged error semantics (NotFound → drop and continue,
    TemporaryFailure → abort, other errors → keep batch in
    sync_pending).
  • Design document doc/sync-performance.md (problem, invariants,
    fallbacks, observability, deferred work); cross-references added to
    architecture.md, integration.md, todo.md, and README.md.

Expected exceptions in new catch blocks: Horde_Imap_Client_Exception
(status prefetch), Horde_ActiveSync_Exception (folder-uid resolution in
the prefetch gather loop, bulk fetch in the exporter).

Test plan

  • New unit tests: ImapAdapterTest (prefetch consumed by ping,
    consume-once, per-mailbox fallback on error),
    StateTest/Sql/ReadonlyLoadStateTest (lock-free load, memo hit
    without SQL, invalidation on mutating load and synckey change),
    StateTest/Sql/SyncCacheSaveTest (dirty merge preserves concurrent
    writers, per-collection removal, whole-property replace,
    skip-when-clean), Connector/ExporterPrefetchTest (bulk consumption,
    per-id fallback, no-bulk-support equivalence, initial-sync uid lists).
  • Full package unit suite: no new failures (4 pre-existing failures
    unrelated to this change).
  • Author deployment soak (iOS device): batched STATUS active,
    SYNC/PING behaviour unchanged, no errors in device logs.
  • Review of the SQL query changes (@ralflang, as requested in RFC: Email sync performance — heartbeat loop state reloads, SyncCache writes, export batching #88).
  • Multi-day soak with several concurrent devices (PING plus parallel
    SYNC on the same folders).

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

🔍 CI Results

Overall: ❌ 12/12 lanes failed

TL;DR: ❌ Quality issues: PHPStan: 243 unique errors in 11 lanes; PHP-CS-Fixer: 29 files.

Summary by PHP Version

PHP dev stable
8.0
8.1
8.2
8.3
8.4
8.5

Quality Metrics

  • PHPUnit: did not run (11 lanes exited non-zero with no test output) ❌
  • PHPStan (advisory): 243 unique errors in 11 lanes ⚠️
  • PHP-CS-Fixer: 29 unique files with issues (of 209 checked) ⚠️
❌ Failed Lanes

php8.0-dev

  • PHPUnit: 0 failures, 0 errors

php8.0-stable

  • PHPUnit: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)
  • PHPStan: ❌ Setup failed (Composer install failed in /tmp/horde-ci/lanes/php8.0-stable/ActiveSync: Your requirements could not be resolved to an installable set of packages.)

php8.1-dev

  • PHPUnit: 0 failures, 0 errors

php8.1-stable

  • PHPUnit: 0 failures, 0 errors

php8.2-dev

  • PHPUnit: 0 failures, 0 errors

php8.2-stable

  • PHPUnit: 0 failures, 0 errors

php8.3-dev

  • PHPUnit: 0 failures, 0 errors

php8.3-stable

  • PHPUnit: 0 failures, 0 errors

php8.4-dev

  • PHPUnit: 0 failures, 0 errors
  • PHP-CS-Fixer: 29 files with issues

php8.4-stable

  • PHPUnit: 0 failures, 0 errors

php8.5-dev

  • PHPUnit: 0 failures, 0 errors

php8.5-stable

  • PHPUnit: 0 failures, 0 errors

CI powered by horde-componentsView full results

Reduce the per-iteration cost of the PING/heartbeat loop and of message
export while preserving foreign-client change detection:

- Batch the IMAP STATUS checks for all pinged email folders into one
  round trip per poll iteration (LIST-STATUS where available) via the
  new optional Driver_Base::prefetchFolderStatus(); prefetched entries
  are consumed once by Imap_Adapter::ping() and fall back per mailbox.
- Add a read-only state load path (loadState(..., ['readonly' => true]))
  used by the poll loop: no collection lock, no garbage collection, no
  SELECT ... FOR UPDATE. State_Sql memoizes decoded state rows per
  folder and synckey, so iterations with an unchanged synckey need no
  SQL at all.
- Honor $dirty in State_Sql::saveSyncCache(): merge only dirty
  properties into the stored cache (per-collection granularity),
  UPDATE-first then INSERT, skip clean saves, and stop logging full
  cache blobs. Fix a missing dirty mark for bodypartprefs in SyncCache.
- Prefetch upcoming email exports in batches of 10 via the new optional
  Driver_Base::getMessagesBulk() in Connector_Exporter_Sync; ids
  missing from a batch fall back to single getMessage() calls with
  unchanged error semantics.

Every optimization is an optional hint with an explicit fallback to the
previous per-item behaviour. Design document: doc/sync-performance.md.

Companion change: horde/core implements prefetchFolderStatus() and
getMessagesBulk() in Horde_Core_ActiveSync_Driver.

Refs #88
Process tracking (review status, soak phases) does not belong in the
long-term roadmap file; without it there are no open points left for
the performance work. The entry lands in the tree together with the
implementation, so "recently completed" is accurate on merge.
@TDannhauer
TDannhauer force-pushed the feat/sync-performance branch from f2dcf5d to e9c153c Compare July 24, 2026 10:01
@TDannhauer
TDannhauer merged commit 61f88b2 into FRAMEWORK_6_0 Jul 24, 2026
1 check failed
ralflang added a commit that referenced this pull request Jul 24, 2026
Release version 3.2.0

Merge pull request #91 from horde/feat/sync-performance
docs(activesync): move #88 entry to recently completed in todo.md
feat(activesync): cut steady-state polling and export cost (#88)
feat(sync): evict ghost items via deferred synthetic deletions
refactor(activesync): centralize client quirks in Device::hasQuirk()
feat(activesync): persist folder UID map across FolderSync resets (#87)
feat(sync): idempotent email import so retries do not duplicate IMAP (#85) (#86)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants